Skip to content

type checker: identify compatible type from reference#8485

Merged
sspaink merged 7 commits intoopen-policy-agent:mainfrom
sspaink:fixtypecheckinginference
Apr 8, 2026
Merged

type checker: identify compatible type from reference#8485
sspaink merged 7 commits intoopen-policy-agent:mainfrom
sspaink:fixtypecheckinginference

Conversation

@sspaink
Copy link
Copy Markdown
Member

@sspaink sspaink commented Apr 3, 2026

resolve: #7273

Better solution then the one I was attempting here: #7273 (comment). My original approach was to refactor unify1 to infer types conditionally and that just made the problem bigger than it had to be.

This approach adding an extra check using the existing unifies to catch compatibility when the type is known is straightforward way to handle this. unify1 does also check for compatibility but because of the infer type functionality it was overwriting the reference incorrectly.

One of the big hurdles I had was trying to understand what unify1 and unifies were doing, hopefully the added comments will make that easier the next time someone looks at this.

The update to unifiesObjectsStatic also resolves the issues: #6751 and #5594, test case added

Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
@netlify
Copy link
Copy Markdown

netlify bot commented Apr 3, 2026

Deploy Preview for openpolicyagent ready!

Name Link
🔨 Latest commit 9255872
🔍 Latest deploy log https://app.netlify.com/projects/openpolicyagent/deploys/69d6afac112521000804ecd3
😎 Deploy Preview https://deploy-preview-8485--openpolicyagent.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

sspaink added 4 commits April 3, 2026 12:00
…ies to be optional, similar to unify1Object

Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
consolidate tests

Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Copy link
Copy Markdown
Member

@anderseknert anderseknert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this is awesome! Very happy to finally see this fixed

I'm quite surprised — bordering on concerned 😄 — this required no updates to existing tests! Did we have nothing to assert past behavior of the compiler for any of the issues mentioned? Wow..

pre := make([]types.Type, len(args))
for i := range args {
pre[i] = env.Get(args[i])
pre[i] = env.GetByValue(args[i].Value)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

@anderseknert
Copy link
Copy Markdown
Member

The update to unifiesObjectsStatic also resolves the issues: #6751 and #5594, test case added

Perhaps add a comment above those tests linking to the issues they verify a fix for.

sspaink added 2 commits April 8, 2026 13:56
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
@sspaink sspaink enabled auto-merge (squash) April 8, 2026 19:43
@sspaink sspaink merged commit 261a4f6 into open-policy-agent:main Apr 8, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Type checker fails to identify type from reference

2 participants